scheduler: decouple the keyranges with different schedulers by bufferflies · Pull Request #9330 · tikv/pd

bufferflies · 2025-05-15T11:56:02Z

What problem does this PR solve?

Issue Number: Close #9302

What is changed and how does it work?

Check List

Tests

Unit test
Integration test

Code changes

Side effects

Possible performance regression
Increased code complexity
Breaking backward compatibility

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
PR to update pingcap/tiup:
Need to cherry-pick to the release branch

Release note

None.

Signed-off-by: 童剑 <[email protected]>

ti-chi-bot · 2025-05-15T11:56:06Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Signed-off-by: 童剑 <[email protected]>

codecov · 2025-05-16T03:55:24Z

Codecov Report

Attention: Patch coverage is 96.17834% with 6 lines in your changes missing coverage. Please review.

Project coverage is 76.18%. Comparing base (49c177a) to head (357c741).
Report is 5 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9330      +/-   ##
==========================================
- Coverage   76.18%   76.18%   -0.01%     
==========================================
  Files         478      479       +1     
  Lines       74794    74988     +194     
==========================================
+ Hits        56984    57129     +145     
- Misses      14282    14314      +32     
- Partials     3528     3545      +17

Flag	Coverage Δ
unittests	`76.18% <96.17%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: 童剑 <[email protected]>

pkg/schedule/schedulers/balance_leader.go

Signed-off-by: 童剑 <[email protected]>

pkg/schedule/schedulers/balance_region.go

rleungx · 2025-05-29T07:46:00Z

pkg/schedule/rangelist/key_range_manager.go

+)
+
+// KeyRangeManager is a manager for key ranges.
+type KeyRangeManager struct {


Seems keyutil is a more proper package?

moved to the new pkg pkg/schedule/rangelist

rleungx · 2025-05-29T07:47:10Z

pkg/schedule/schedulers/balance_leader.go

 func (s *balanceLeaderScheduler) transferLeaderOut(solver *solver, collector *plan.Collector) *operator.Operator {
-	solver.Region = filter.SelectOneRegion(solver.RandLeaderRegions(solver.sourceStoreID(), s.conf.getRanges()),
+	rs := s.conf.getRanges()
+	if IsDefaultKeyRange(rs) {


How about the hot region scheduler?

Hot region scheduler can schedule all the regions, not restricting this limit.

Signed-off-by: 童剑 <[email protected]>

bufferflies · 2025-06-10T03:20:47Z

ping @rleungx again

rleungx · 2025-06-10T06:01:17Z

pkg/schedule/keyrange/manager.go

+	for _, r := range rs {
+		s.sortedKeyRanges.Append(r.StartKey, r.EndKey)
+	}
+	s.sortedKeyRanges.SortAndDeduce()


For every time we append, we need to sort?

done， using adjust function to check the two regions

Signed-off-by: 童剑 <[email protected]>

rleungx · 2025-06-11T06:42:55Z

pkg/utils/keyutil/util.go


+// MaxKeyWithBoundary returns the bigger key for the given keys.
+// it is different from MaxKey, if the key is empty and the boundary is right, the empty key is biggest,
+func MaxKeyWithBoundary(a, b []byte, opt boundary) []byte {


How about changing to MaxStartKey/MinEndKey? The boundary is useless.

rleungx · 2025-06-11T06:45:44Z

pkg/utils/keyutil/keyrange.go

+	for i := 1; i < len(rs.krs); i++ {
+		last := res[len(res)-1]
+		if last.IsAdjacent(rs.krs[i]) {
+			last.StartKey = MinKeyWithBoundary(last.StartKey, rs.krs[i].StartKey, left)


It has already been sorted, no need to compare?

rleungx · 2025-06-11T06:47:23Z

pkg/schedule/schedulers/balance_leader.go

+	rs := s.conf.getRanges()
+	if IsDefaultKeyRange(rs) {
+		km := solver.GetKeyRangeManager()
+		rs = km.GetNonOverlappingKeyRanges(&rs[0])


Will it slow down the balance leader's speed?

If one balance-keyrange job is running, the pick ranges of the scheduling regions become small. Otherwise, the rs is the same with the master; this function does not influence the select the source region.

rleungx · 2025-06-11T06:49:05Z

pkg/schedule/schedulers/balance_region.go

 	solver.Step++
 	var sourceIndex int

+	rs := s.conf.Ranges


What if we change the range manually?

The config can only be changed by the scatter range scheduler. This scatter scheduler only picks the given key-range regions, there is no any conflict

I think this config is exposed. We can set them through pd-ctl.

Yes, I used the scheduler name to distinguish the scheduler from the scatter-scheduler

Signed-off-by: 童剑 <[email protected]>

rleungx · 2025-06-19T07:45:07Z

pkg/schedule/schedulers/balance_leader.go

+	rs := s.conf.getRanges()
+	if s.GetName() == types.BalanceLeaderScheduler.String() {
+		km := solver.GetKeyRangeManager()
+		rs = km.GetNonOverlappingKeyRanges(&rs[0])


How about skipping it if there is no job?

rleungx

The rest LGTM

ti-chi-bot · 2025-06-19T10:36:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lhy1024, rleungx

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [lhy1024,rleungx]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2025-06-19T10:36:24Z

[LGTM Timeline notifier]

Timeline:

2025-06-09 03:02:19.409732585 +0000 UTC m=+237717.638047833: ☑️ agreed by lhy1024.
2025-06-19 10:36:22.91088544 +0000 UTC m=+354435.634064424: ☑️ agreed by rleungx.

Signed-off-by: 童剑 <[email protected]>

…range

bufferflies added 7 commits May 9, 2025 14:30

inferface

e1ec26d

Signed-off-by: 童剑 <[email protected]>

inferface

a39754d

Signed-off-by: 童剑 <[email protected]>

move

fdd5a97

Signed-off-by: 童剑 <[email protected]>

conflict

1270f87

Signed-off-by: 童剑 <[email protected]>

lint

fe030a0

Signed-off-by: 童剑 <[email protected]>

scheduler

1a2d2ab

Signed-off-by: 童剑 <[email protected]>

conflict

8d91e0d

Signed-off-by: 童剑 <[email protected]>

ti-chi-bot bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels May 15, 2025

ti-chi-bot bot added dco-signoff: yes Indicates the PR's author has signed the dco. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 15, 2025

bufferflies force-pushed the feature/decouple_keyrange branch 2 times, most recently from 7fa0925 to 808b7a1 Compare May 16, 2025 03:40

delete range

dbff105

Signed-off-by: 童剑 <[email protected]>

bufferflies force-pushed the feature/decouple_keyrange branch from 808b7a1 to dbff105 Compare May 16, 2025 03:46

conflict

5bb9eb5

Signed-off-by: 童剑 <[email protected]>

bufferflies force-pushed the feature/decouple_keyrange branch from 885ffe7 to 51f06d1 Compare May 22, 2025 12:24

add more test for balance range

01f2334

Signed-off-by: 童剑 <[email protected]>

bufferflies force-pushed the feature/decouple_keyrange branch from 51f06d1 to 01f2334 Compare May 27, 2025 07:48

bufferflies marked this pull request as ready for review May 27, 2025 07:48

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 27, 2025

lhy1024 reviewed May 27, 2025

View reviewed changes

pkg/schedule/schedulers/balance_leader.go Outdated Show resolved Hide resolved

IsDefaultKeyRange

7b54827

Signed-off-by: 童剑 <[email protected]>

rleungx reviewed May 29, 2025

View reviewed changes

pkg/schedule/schedulers/balance_region.go Show resolved Hide resolved

rleungx reviewed May 29, 2025

View reviewed changes

move new pkg

338ca04

Signed-off-by: 童剑 <[email protected]>

ti-chi-bot bot added the approved label Jun 9, 2025

rleungx reviewed Jun 10, 2025

View reviewed changes

use Adjacent

02af066

Signed-off-by: 童剑 <[email protected]>

ti-chi-bot bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 10, 2025

default timeout is 30m

7ce9e71

Signed-off-by: 童剑 <[email protected]>

rleungx reviewed Jun 11, 2025

View reviewed changes

address comment

9db3d05

Signed-off-by: 童剑 <[email protected]>

bufferflies force-pushed the feature/decouple_keyrange branch from 6e774db to 9db3d05 Compare June 18, 2025 11:15

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 18, 2025

use scheduler name to check the balance leader/region

255029d

Signed-off-by: 童剑 <[email protected]>

bufferflies force-pushed the feature/decouple_keyrange branch from e93b5b6 to 255029d Compare June 19, 2025 07:04

rleungx reviewed Jun 19, 2025

View reviewed changes

rleungx approved these changes Jun 19, 2025

View reviewed changes

ti-chi-bot bot added the lgtm label Jun 19, 2025

ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jun 19, 2025

bufferflies force-pushed the feature/decouple_keyrange branch from 3a92d59 to 3706344 Compare June 19, 2025 11:13

return direct if kr is empty

394843c

Signed-off-by: 童剑 <[email protected]>

bufferflies force-pushed the feature/decouple_keyrange branch from 7bda573 to 394843c Compare June 20, 2025 01:08

Merge branch 'master' of github.com:tikv/pd into feature/decouple_key…

357c741

…range

bufferflies force-pushed the feature/decouple_keyrange branch from 854a502 to 357c741 Compare June 20, 2025 02:41

ti-chi-bot bot merged commit 206b56e into tikv:master Jun 20, 2025
26 checks passed

Conversation

bufferflies commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

Uh oh!

ti-chi-bot bot commented May 15, 2025

Uh oh!

codecov bot commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bufferflies commented Jun 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bufferflies Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rleungx Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rleungx left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Jun 19, 2025

Uh oh!

ti-chi-bot bot commented Jun 19, 2025

[LGTM Timeline notifier]

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bufferflies commented May 15, 2025 •

edited

Loading

codecov bot commented May 16, 2025 •

edited

Loading

bufferflies Jun 10, 2025 •

edited

Loading

rleungx Jun 18, 2025 •

edited

Loading